Skip to content

perf(gc): stop re-marking a heap where nothing dies — yield-adaptive major pacing + the evacuation move-hook mutex - #7733

Merged
proggeramlug merged 7 commits into
mainfrom
gc/p5-retain-live-set
Aug 9, 2026
Merged

perf(gc): stop re-marking a heap where nothing dies — yield-adaptive major pacing + the evacuation move-hook mutex#7733
proggeramlug merged 7 commits into
mainfrom
gc/p5-retain-live-set

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

gc-handoff/bench/retain.ts builds a 3 M-element array of {a, b} records,
keeps every one of them live, then sums a field. Nothing is ever garbage,
and Perry spent 1.26 s of a 1.31 s run inside the collector — 96%. Node
does it in 0.14 s.

Two independent causes, both measured on the pinned quiet M1 mini against
origin/main @ c156f8a41.

1. Half the pause was two full mark-sweeps that reclaimed 4 MB between them

PERRY_GC_TRACE=1 ./n_retain on main:

cycle kind pause arena in-use before → after
3 full 160.6 ms 67 → 63 MB
6 full 483.5 ms 204 → 204 MB

arena_growth_full_escalation_due escalates a minor to a full once arena
live-bytes pass K× (K = 2) the live set measured after the last full. Its own
doc-comment claimed a "legitimately large stable live set (retain-style) does
not over-escalate — its arena hovers near its own baseline". retain's live
set is not stable, it grows: every doubling crosses the threshold, and each
resulting full marks a heap where nothing has died. 644 ms of pause for 4 MB,
and the second one moved arena in-use by exactly zero.

So price a full by what it reclaims. A full that shrinks arena in-use by less
than 20% shifts the next escalation threshold left by one — capped at 2, so the
multiplier tops out at 8× — and a productive full resets the shift to 0 in one
step. The yield is measured on the same metric the escalation gate reads,
so the two cannot disagree about whether a full helped, and it is scoped to
escalated fulls: an explicit gc() never moves the backoff, or a
gc()-in-a-loop test would drive it to the cap. Old-gen garbage is unaffected
old_reclaim_pressure_due still forces its own full.

2. Every evacuated object took a process-global mutex to hash an empty map

object_static_prototype_owner_moved — the ObjectOverflowFields move hook,
run once per moved object — took a Mutex<HashMap<usize, u64>> and ran a
SipHash remove against the residual Object.setPrototypeOf registry.

That registry is empty in any program that never re-prototypes a
non-meta-capable owner, and a latch already says so: OBJECT_PROTOTYPES_NONEMPTY,
stored Release before the insert, and read by both siblings
(object_static_prototype, prune_dead_object_prototype_owners). The move
hook was the one reader that skipped it, so a 3 M-record promotion paid 2.5 M
lock/unlock pairs and 2.5 M hash probes against an empty map. It is why a
single-threaded benchmark profiled with pthread_mutex_lock and
std::hash::random::RandomState in its top ten.

Isolated without a second build by retain_latched.ts, a twin that latches the
registry through a RegExp owner: on this binary retain is 0.81 s and
retain_latched 0.84 s, and on main the two are 1.31/1.32 s — the latch fix
is worth 0.03 s, and the control shows the A/B is measuring the right thing.

The bug this nearly shipped as

The first cut recorded the pre-full arena reading at the two
gc_start_budgeted_cycle_for_pressure call sites. Both correct, both the wrong
sites: the escalation the shipped safepoint path actually takes lives in
gc::gc_collect_minor_with_trigger_inner, so the reading was never recorded,
update_major_pacing_backoff early-returned on every cycle, and the change
measured 1.31 → 1.28 s — the mutex fix alone — with every test green.

The recording now happens inside arena_growth_full_escalation_due on the
true verdict, so a call site added later is priced by construction. The GC
trace gained a major_pacing block (baseline_bytes, backoff_shift,
escalate_above_bytes) so the subject can be asserted live rather than
inferred from a green run — that block is what showed the backoff sitting at 0
through all seven cycles.

One full is the right answer, not zero

Turning major pacing off entirely (PERRY_GC_MAJOR_PACING_FLOOR_MB=0, so
arena_growth_full_escalation_due is a constant false) makes retain
slower, not faster: 1.14 s and 390 MB peak RSS, against 0.81 s / 342 MB
with the backoff and 1.31 s / 373 MB on main. The array's abandoned backing
buffers are the one thing on this workload that only a full reclaims, so the
surviving full earns its 161 ms — the two on main did not. The backoff lands
between the extremes rather than at one of them, which is the point.

The discriminator, checked on the workload it must NOT touch

tree.ts runs 40 escalated fulls (all arena_bytes) in a 1.64 s run,
562 ms of pause. Every one of them takes arena in-use from ~41 MB to ~5 MB —
an 88% yield. The backoff reads 0 through all forty and tree is
bit-for-bit unchanged. retain's two fulls yield 5.9% and 0.0% and the backoff
moves on the first one. That is the whole heuristic, observed on both sides.

Measurements — quiet M1 mini, best of 5, absolute seconds

bench node main this PR Δ protected floor
retain 0.14 1.31 0.81 −38%
retain_wide 0.16 1.74 1.35 −22%
retain_prealloc 1.70 1.45 −15%
churn_read 0.08 0.02 0.02 ≤ 0.03 ✓
cycles 0.07 0.19 0.19 ≤ 0.20 ✓
deeplist 0.09 0.31 0.30 −0.01 ≤ 0.35 ✓
tree 0.45 1.64 1.64 ≤ 1.70 ✓
tree_wide 0.90 2.10 2.10 ≤ 2.20 ✓
churn 0.45 0.45 ≤ 0.46 ✓
churn_alloc 0.14 0.41 0.41 ≤ 0.42 ✓
push_cls 0.13 0.40 0.40 ≤ 0.40 ✓
push_num 0.11 0.17 0.17

Peak RSS goes down, not up: retain 373 → 342 MB, retain_wide 538 → 470 MB,
retain_prealloc 416 → 408 MB. Every stdout is byte-identical to
node --experimental-strip-types.

GC traces, before → after:

cycles fulls total pause max pause
retain 7 → 6 2 → 1 1273 → 728 ms 483 → 201 ms
retain_wide 10 → 9 3 → 2 1690 → 1257 ms 687 → 495 ms

gc_ratchet (shared_ci), measured on both arms on the same host: main
currently fails four 12_large_live_set cells against the pinned baseline
(heap_total_bytes +19.05%, promoted_objects +10.79%, promoted_bytes
+11.17%, freed_bytes +10.86%); this PR returns all four to ok and drops
that probe's peak RSS 191 → 170 MB. Its post-gc() heap_used_bytes goes
33.6 → 50.4 MB, still under the 51.7 MB baseline and still reported as an
improvement. Every other cell's verdict is identical on both arms, including
the 04_dead_after_deep_stack / 11_collect_at_depth regressions that are
already red on main and are byte-identical between arms.

gc-handoff/apps/iso_miss.ts prints checksum 437840 misses 0.

Test status

cargo test --release -p perry-runtime --no-fail-fast: 1961 pass, 3 fail.
The three are gc::tests::runtime_roots::generator_attach_prototype::*, and
they are pre-existing on main — a test binary built at c156f8a41 fails
the same three, and they also fail on this branch with major pacing switched
off entirely (PERRY_GC_MAJOR_PACING_FLOOR_MB=0, which makes
arena_growth_full_escalation_due a constant false). All three are
live-subject assertions about the arena trigger arming / safepoint deferral,
not about which collection kind runs.

Not fixed here, measured and named

retain is still ~5.8× node. The residue is one number: ~220 ns of
per-object bookkeeping for every promoted object
, 2.5 M of them on this
workload — arena_alloc_gc_old, layout_transfer,
old_page_account_promoted_object, the move hooks, and two page-generation
classifications per visited slot. The structural answer is V8-style whole-page
promotion (relabel a nearly-all-live Eden block as old-gen instead of
evacuating it object by object), which needs none of that per-object work.

Second on the list: gc::verify::restore_surviving_dirty_coverage walks
every slot of a parent on a pre-cycle dirty page, where the scan it repairs
(scan_dirty_object_slots) walks only the slots on dirty pages — 8.8% of a
pure-retain profile, re-walking the whole 3 M-element backing store on every
minor.

Summary by CodeRabbit

  • Performance

    • Improved garbage-collection pacing by adapting full-collection frequency based on reclaimed memory.
    • Reduced overhead when no prototype registry entries exist.
    • Productive collections promptly restore normal pacing, while low-yield collections apply bounded backoff.
  • Diagnostics

    • Added major_pacing data to garbage-collection diagnostic output, including thresholds and current pacing state.
  • Documentation

    • Added release notes describing the GC performance improvements, benchmark results, and remaining optimization areas.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 99133434-f6de-4019-9754-93337db694d1

📥 Commits

Reviewing files that changed from the base of the PR and between 08c7f07 and 89675c0.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • CLAUDE.md
  • Cargo.toml

📝 Walkthrough

Walkthrough

The PR adds yield-adaptive backoff for unproductive escalated full collections, exposes major-pacing values in GC telemetry, adds pacing tests, skips prototype-registry work when the registry is empty, and updates the package version.

Changes

Major-GC pacing

Layer / File(s) Summary
Yield-adaptive escalation policy
crates/perry-runtime/src/gc/policy.rs
Full collections record pre-collection usage, adjust a bounded backoff shift from reclaimed bytes, and reset the shift after productive reclamation.
Pacing telemetry and validation
crates/perry-runtime/src/gc/telemetry.rs, crates/perry-runtime/src/gc/tests/triggers.rs, changelog.d/7733-retain-live-set-major-pacing.md, CLAUDE.md, Cargo.toml
GC JSON reports major_pacing values. Tests cover defaults, thresholds, caps, resets, and declined escalation recording. The changelog and package documentation describe the changes. The workspace version changes to 0.5.1427.

Prototype registry guard

Layer / File(s) Summary
Empty-registry evacuation path
crates/perry-runtime/src/object/prototype_chain.rs, changelog.d/7733-retain-live-set-major-pacing.md
Prototype ownership migration skips registry locking and hashing when the nonempty latch is clear.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant GCTrigger
  participant GCPacingPolicy
  participant FullCollector
  participant GCTelemetry
  GCTrigger->>GCPacingPolicy: evaluate escalation threshold
  GCPacingPolicy->>GCPacingPolicy: record pre-full arena usage
  GCPacingPolicy->>FullCollector: start escalated full collection
  FullCollector->>GCPacingPolicy: report reclaimed bytes
  GCPacingPolicy->>GCTelemetry: provide major_pacing values
Loading

Possibly related PRs

Suggested reviewers: jdalton

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes both primary changes: yield-adaptive major-GC pacing and the evacuation mutex optimization.
Description check ✅ Passed The description provides a detailed summary, concrete changes, benchmark results, test status, and known limitations, despite omitting some template headings.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch gc/p5-retain-live-set

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

proggeramlug pushed a commit that referenced this pull request Aug 9, 2026
@proggeramlug
proggeramlug marked this pull request as ready for review August 9, 2026 20:23

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-runtime/src/gc/policy.rs`:
- Around line 1572-1577: The major_pacing_snapshot boundary omits the configured
floor, causing escalate_above_bytes to report zero or too-low values when
escalation is still floor-gated. Update major_pacing_snapshot to include
floor_bytes in the effective threshold, or expose floor_bytes separately and
adjust the telemetry contract; add coverage for a nonzero floor with zero and
low baselines, preserving existing behavior for baselines above the floor.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1156f50d-d65c-4001-aa4a-c7f0041a3e9c

📥 Commits

Reviewing files that changed from the base of the PR and between a8d1a77 and 08c7f07.

📒 Files selected for processing (5)
  • changelog.d/7733-retain-live-set-major-pacing.md
  • crates/perry-runtime/src/gc/policy.rs
  • crates/perry-runtime/src/gc/telemetry.rs
  • crates/perry-runtime/src/gc/tests/triggers.rs
  • crates/perry-runtime/src/object/prototype_chain.rs

Comment on lines +1572 to +1577
pub(super) fn major_pacing_snapshot() -> (usize, u32, usize) {
let (_floor, growth_num) = major_pacing_config();
let baseline = GC_LAST_FULL_ARENA_IN_USE_BYTES.with(|bytes| bytes.get());
let shift = major_pacing_backoff_shift();
let threshold = baseline.saturating_mul(growth_num.saturating_mul(1usize << shift));
(baseline, shift, threshold)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Include the floor in the reported escalation boundary.

major_pacing_snapshot reports only baseline × growth. The predicate also rejects values below floor_bytes at Lines 2582-2587. When baseline == 0, this snapshot reports 0, although escalation cannot occur until the arena reaches the configured floor.

telemetry.rs exports this value as major_pacing.escalate_above_bytes. Return the effective boundary, or emit floor_bytes separately and update the field contract. Add coverage for a nonzero floor with both a zero and a low baseline.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-runtime/src/gc/policy.rs` around lines 1572 - 1577, The
major_pacing_snapshot boundary omits the configured floor, causing
escalate_above_bytes to report zero or too-low values when escalation is still
floor-gated. Update major_pacing_snapshot to include floor_bytes in the
effective threshold, or expose floor_bytes separately and adjust the telemetry
contract; add coverage for a nonzero floor with zero and low baselines,
preserving existing behavior for baselines above the floor.

@proggeramlug
proggeramlug force-pushed the gc/p5-retain-live-set branch from 08c7f07 to 89675c0 Compare August 9, 2026 21:10
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Merging as v0.5.1427

Two independent changes, and I checked the one that could hurt in production rather than in a benchmark.

The unbounded-growth question, verified in code

Delaying a full GC is how RSS runs away, so the backoff needed three properties and has all three:

  1. The cap is enforced, not just described. MAJOR_PACING_PRODUCTIVE_YIELD_PCT = 20, shift capped at 2 so the multiplier tops out at 8×, with the comment stating the point: "a long run of low-yield fulls cannot disable arena-growth pacing outright." The tests pin both ends — assert_eq!(major_pacing_backoff_shift(), 2, "the shift is capped") and the one-step reset to 0 on a productive full.
  2. The safety valve is genuinely independent. old_reclaim_pressure_due (policy.rs:1319) is a pure function of old_in_use and baseline against thresholds; GC_MAJOR_PACING_BACKOFF_SHIFT appears nowhere in it. Old-gen garbage still forces its own full whatever the backoff is doing.
  3. So the worst case is 8× the post-full live set, bounded — not unbounded. That is the honest cost and it is the right trade against 644 ms of pause reclaiming 4 MB.

Scoping it to escalated fulls is also right: an explicit gc() never moves the backoff, so a gc()-in-a-loop test can't drive it to the cap.

The diagnosis corrects a doc that was confidently wrong

arena_growth_full_escalation_due's own comment claimed a "legitimately large stable live set (retain-style) does not over-escalate — its arena hovers near its own baseline." retain's live set is not stable, it grows: every doubling crosses the threshold, and each resulting full marks a heap where nothing has died. The second full moved arena in-use by exactly zero.

Pricing a full by what it reclaims, measured on the same metric the escalation gate reads, is what stops the two from disagreeing about whether a full helped. That detail is easy to get wrong and would produce oscillation.

The move-hook mutex

object_static_prototype_owner_moved runs once per moved object and took a process-global Mutex<HashMap<usize,u64>> to SipHash-remove against a registry that is empty in every program that never calls Object.setPrototypeOf. Removing a lock from the per-object evacuation path is the kind of fix that only shows up when evacuation is actually happening — which, since #7721, it now is.

Worth watching, given this repo's history: #7510 found that one immortal side-table entry nullified every is_empty() fast path. If a single long-lived setPrototypeOf owner can pin this registry non-empty, the fast path is off for the whole process. Not a blocker — the slow path is what shipped before — but worth a follow-up check.

Gates 21/21.

@proggeramlug
proggeramlug merged commit 6bfb13a into main Aug 9, 2026
14 of 16 checks passed
@proggeramlug
proggeramlug deleted the gc/p5-retain-live-set branch August 9, 2026 21:17
proggeramlug added a commit that referenced this pull request Aug 10, 2026
…7737) (#7740)

* fix(gc): release the prototype-registry latch when a prune drains it (#7737)

The latch was one-way, so a single Object.setPrototypeOf anywhere in a
process permanently disabled #7733's per-evacuated-object fast path. The
set moves under the mutex so the clear cannot race an in-flight insert.

Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix

* chore: bump version to 0.5.1432

Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix

* style: cargo fmt

Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix

---------

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug pushed a commit that referenced this pull request Aug 10, 2026
…7733 follow-up)

`major_pacing_snapshot` recomputed the escalation boundary as
`baseline x growth` and discarded the floor (`let (_floor, growth_num) = ...`),
while `arena_growth_full_escalation_due` also rejects every reading below that
floor. Wherever the floor dominated the two disagreed -- most starkly before the
first full, where the trace reported `0` ("escalates at any size") for a
collector that escalates at 32 MB.

That snapshot exists precisely so the pacing subject can be asserted live in the
GC trace, so a probe that misreports its own subject is worse than none.

There is now one definition of the boundary
(`major_pacing_escalation_threshold_bytes`): the predicate is literally
`in_use >= it`, and the snapshot reports it verbatim, floor included. The trace
key follows the semantics -- `escalate_at_or_above_bytes`, `null` when
`PERRY_GC_MAJOR_PACING_FLOOR_MB=0` disables pacing outright.

Also: the ZealGuard test asserted the arm was taken and only narrated that it
was released, so a Drop that stopped releasing it would have left every later
test in the binary on the poll's slow path with the test still green.

Claude-Session: https://claude.ai/code/session_015JgLM9UWGa6WAMix7CvhQJ
proggeramlug added a commit that referenced this pull request Aug 10, 2026
… the ZealGuard release becomes an assertion (#7729, #7733, #7735 review follow-ups) (#7739)

* fix(gc): the pacing snapshot reports the boundary the predicate uses (#7733 follow-up)

`major_pacing_snapshot` recomputed the escalation boundary as
`baseline x growth` and discarded the floor (`let (_floor, growth_num) = ...`),
while `arena_growth_full_escalation_due` also rejects every reading below that
floor. Wherever the floor dominated the two disagreed -- most starkly before the
first full, where the trace reported `0` ("escalates at any size") for a
collector that escalates at 32 MB.

That snapshot exists precisely so the pacing subject can be asserted live in the
GC trace, so a probe that misreports its own subject is worse than none.

There is now one definition of the boundary
(`major_pacing_escalation_threshold_bytes`): the predicate is literally
`in_use >= it`, and the snapshot reports it verbatim, floor included. The trace
key follows the semantics -- `escalate_at_or_above_bytes`, `null` when
`PERRY_GC_MAJOR_PACING_FLOOR_MB=0` disables pacing outright.

Also: the ZealGuard test asserted the arm was taken and only narrated that it
was released, so a Drop that stopped releasing it would have left every later
test in the binary on the poll's slow path with the test still green.

Claude-Session: https://claude.ai/code/session_015JgLM9UWGa6WAMix7CvhQJ

* docs(gc): changelog fragment for #7739; keep the snapshot testable without diagnostics

Claude-Session: https://claude.ai/code/session_015JgLM9UWGa6WAMix7CvhQJ

* chore: bump version to 0.5.1433

Claude-Session: https://claude.ai/code/session_01Y1QZ5wUP9gRSwpiweT4Wix

---------

Co-authored-by: Ralph Küpper <ralph@skelpo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant